W-1 [single, Reviewer A] · 🟢High — Re-entrant enterEditMode silently overwrites the original snapshot

lib/page/dashboard/providers/dashboard_edit_mode_provider.dart — enterEditMode():

Future<void> enterEditMode() async {
  await ref.read(uspLayoutPreferencesProvider.notifier).initialized;
  final controller = ref.read(uspSliverDashboardControllerProvider);
  final layoutSnapshot = controller.exportLayout();  // ← re-captures already-modified grid
  state = DashboardEditState(
    isEditing: true,
    layoutSnapshot: layoutSnapshot,  // ← unconditionally overwrites original baseline
    ...
  );
}

No if (state.isEditing) return guard. A double-tap or gesture race calls enterEditMode() while already editing, re-capturing the current (already-modified) layout as the new "original" snapshot. Tapping Cancel or navigating away then reverts to the mid-edit state, not the true pre-edit baseline.

Fix: if (state.isEditing) return; as the first line.

W-2 [single, Reviewer A] · 🟡Med — TOCTOU: onExit fires in async gap before isEditing=true

enterEditMode() sets isEditing=true only after await initialized. If the user taps Edit then immediately navigates away before initialized resolves, onExit reads isEditing=false and skips cancelEditMode(). enterEditMode then resumes and sets isEditing=true — the controller is permanently stuck in edit mode on a different page.

Fix: Set state = state.copyWith(isEditing: true) before the await to claim the slot early; onExit will then always see the correct state.

W-3 [single, Reviewer A] · 🟡Med — No try-catch in onExit; exception leaves stale isEditing=true

lib/route/route_usp_dashboard.dart:onExit:

onExit: (context, state) async {
  if (editState.isEditing) {
    await container.read(dashboardEditModeProvider.notifier).cancelEditMode(); // unguarded
  }
  return true;
}

cancelEditMode() calls saveLayout() and restoreSnapshot(), both of which can throw (SharedPreferences failure, platform exception). An unhandled exception leaves dashboardEditModeProvider.isEditing=true permanently; the next dashboard visit has stale state.

Fix: Wrap in try-catch and force-reset provider state on error.

W-4 [single, Reviewer B] · 🟢High — Silent discard diverges from showUnsavedAlert pattern

Every other editable route in the codebase (uspInstantSafety, uspDhcpDetail, uspWifiSettings, etc.) uses enableDirtyCheck: true / showUnsavedAlert before discarding unsaved changes. The dashboard onExit silently calls cancelEditMode() without prompting. A user who navigated away accidentally after rearranging the dashboard loses all work without warning.

Fix: Either use showUnsavedAlert(context) before cancelEditMode(), or add an explicit code comment documenting the intentional silent-discard policy.

W-5 [single, Reviewer B] · 🟡Med — save=false naming is inverted-boolean trap

exitEditMode({bool save = true}) — the save=false branch calls controller.importLayout(snapshot) and then still calls saveLayout() (persists the reverted snapshot to disk). The parameter name says "don't save" but a save operation executes inside the !save branch.

Fix: Rename to exitEditMode({bool revert = false}), or expose only cancelEditMode() / commitEditMode() as the public API and make exitEditMode private.

• Full migration confirmed: Synthesizer re-read the complete diff — all 7 _isEditMode references in usp_sliver_dashboard_view.dart (_buildHeader, _buildSliverDashboard ×2, trashBuilder ×2, _buildItemWidget ×2, _openLayoutSettings) are updated to use ref.watch(dashboardEditModeProvider).isEditing. No stale local-state references remain.
• Architecture compliance: Three-layer rules respected. Provider imports only models and sibling providers; view imports only the new provider. No codegen model imports. No custom widgets introduced.
• Provider rules: NotifierProvider (non-autoDispose, correct L1). All ref.read inside notifier methods (correct L2 pattern). No uspMutationLockProvider needed (local preference writes, not USP API mutations). PreservableContract not redefined.
• Route guard idiom: ProviderScope.containerOf(context) is the idiomatic pattern for accessing Riverpod providers in go_router callbacks outside widget tree.
• Unit test coverage: dashboard_edit_mode_provider_test.dart (172 lines) covers state model, enter, exit (save/revert), cancel, and multi-cycle stability. Solid provider-level unit coverage.
• No security issues: No hardcoded secrets, injection surfaces, permission bypasses, or unsafe data handling.

All 7 Round-1 findings are fully resolved in this diff:

• W-1 Re-entrant enterEditMode — ✅ if (state.isEditing) return; guard added at dashboard_edit_mode_provider.dart:50.
• W-2 TOCTOU race — ✅ state = const DashboardEditState(isEditing: true) set before the await (line 54); post-await if (!state.isEditing) return; bail-out added (line 60).
• W-3 No try-catch in onExit — ✅ try/catch wraps cancelEditMode() in route_usp_dashboard.dart with logger.e on failure.
• W-4 Silent-discard vs. showUnsavedAlert — ✅ Intentional-policy comment added in route_usp_dashboard.dart:21–29.
• W-5 save=false inverted-boolean naming — ✅ API renamed to commitEditMode() / cancelEditMode() / _exitEditMode({required bool revert}).
• S-1 List<dynamic>? weak type — ✅ Upgraded to List<Map<String, dynamic>>?.
• S-2 Missing integration tests — ✅ New tests added: commitEditMode-preserves-changes, W-1 re-entrant regression, W-2 TOCTOU cancel-during-async-gap.
• PeterJhong comment (_openLayoutSettings reset/preset reverts change) — ✅ _openLayoutSettings now calls commitEditMode() for reset/preset_changed (keeps the applied change instead of reverting); toggle_off branch removed (was dead code — settings panel never emitted this string).
• linksys_route.dart deletion — ✅ Confirmed safe: file was never imported anywhere; active LinksysRoute lives in route_model.dart using PreservableContract.

W-1 [single, Reviewer A] · 🟢High — Partial revert on saveLayout() exception leaves layout/prefs inconsistent

lib/page/dashboard/providers/dashboard_edit_mode_provider.dart:82–101

Future<void> _exitEditMode({required bool revert}) async {
  final controller = ref.read(uspSliverDashboardControllerProvider);
  try {
    if (revert) {
      if (state.layoutSnapshot != null) {
        controller.importLayout(state.layoutSnapshot!);   // Step 1: in-memory revert OK
        await ref
            .read(uspSliverDashboardControllerProvider.notifier)
            .saveLayout();                                // Step 2: can throw
      }
      if (state.prefsSnapshot != null) {
        await ref
            .read(uspLayoutPreferencesProvider.notifier)
            .restoreSnapshot(state.prefsSnapshot!);      // Step 3: skipped on Step 2 throw
      }
    }
  } finally {
    controller.setEditMode(false);
    state = const DashboardEditState();                  // snapshots cleared, no retry possible
  }
}

If saveLayout() throws (SharedPreferences I/O failure), execution jumps to finally, skipping restoreSnapshot(). Resulting state:

The user sees the layout reverted but prefs remain in the edited state. On app restart the combination can produce inconsistent UI. The finally comment only guarantees "edit mode won't be stranded" — it does not protect layout/prefs consistency.

Trigger: Any I/O error in saveLayout() during a cancel/nav-away (uncommon, not impossible).

Fix:

try {
  if (revert) {
    if (state.layoutSnapshot != null) {
      controller.importLayout(state.layoutSnapshot!);
      try {
        await ref.read(uspSliverDashboardControllerProvider.notifier).saveLayout();
      } catch (e, s) {
        logger.e('[Dashboard] saveLayout failed during revert; continuing to restore prefs',
            error: e, stackTrace: s);
        // continue — restoreSnapshot must still run to keep prefs consistent
      }
    }
    if (state.prefsSnapshot != null) {
      await ref.read(uspLayoutPreferencesProvider.notifier).restoreSnapshot(state.prefsSnapshot!);
    }
  }
} finally {
  controller.setEditMode(false);
  state = const DashboardEditState();
}

W-2 [single, Reviewer B] · 🟢High — void async in view-layer wrappers silently discards exceptions

lib/page/dashboard/views/usp_sliver_dashboard_view.dart:115–121

void _commitEditMode() async {
  await ref.read(dashboardEditModeProvider.notifier).commitEditMode();
}

void _cancelEditMode() async {
  await ref.read(dashboardEditModeProvider.notifier).cancelEditMode();
}

In Dart, void async functions discard the Future; any uncaught exception surfaces only through FlutterError.onError or the zone's uncaught-error handler. In production, errors in commitEditMode() / cancelEditMode() become invisible. The _exitEditMode finally block guarantees state is reset, so no functional regression — but failure observability is lost.

This pattern (_enterEditMode already uses it) is an existing convention in the view; the new methods extend it. Not blocking, but an opportunity to improve diagnostics.

Fix (non-breaking):

void _commitEditMode() {
  ref.read(dashboardEditModeProvider.notifier).commitEditMode()
    .catchError((e, s) => logger.e('[Dashboard] commitEditMode failed', error: e, stackTrace: s));
}

void _cancelEditMode() {
  ref.read(dashboardEditModeProvider.notifier).cancelEditMode()
    .catchError((e, s) => logger.e('[Dashboard] cancelEditMode failed', error: e, stackTrace: s));
}

• Core logic is correct end-to-end — re-entrant guard, TOCTOU fix, API rename, _openLayoutSettings commit path, toggle_off dead-code removal all confirmed correct by reading actual source.
• Architecture compliance — NotifierProvider (non-autoDispose), all ref.read inside notifier, no USP mutation lock needed (pure local-prefs writes), no codegen imports, no custom widgets. Fully compliant with three-layer and provider rules.
• finally block guarantees no stuck edit-mode — even if revert operations partially fail, the controller and provider state are always reset. The main gap (W-1) is about data consistency, not edit-mode being stranded.
• Test suite is comprehensive at the provider level — re-entrant, TOCTOU, commit-preserves-changes, multi-cycle, cancel-revert all covered. Strong regression prevention.
• linksys_route.dart deletion confirmed safe — route_model.dart is the sole active LinksysRoute definition; the deleted file was never imported; no duplicate-class risk.
• No security issues — no hardcoded secrets, no injection surfaces, no permission bypasses, no JNAP/USP response handling changes.

W-1 [single, Reviewer A, CARRY-OVER] · 🟢High — saveLayout() exception skips restoreSnapshot()

lib/page/dashboard/providers/dashboard_edit_mode_provider.dart:82–101

Future<void> _exitEditMode({required bool revert}) async {
  final controller = ref.read(uspSliverDashboardControllerProvider);
  try {
    if (revert) {
      if (state.layoutSnapshot != null) {
        controller.importLayout(state.layoutSnapshot!);   // Step 1: in-memory revert OK
        await ref
            .read(uspSliverDashboardControllerProvider.notifier)
            .saveLayout();                                // Step 2: can throw
      }
      if (state.prefsSnapshot != null) {
        await ref
            .read(uspLayoutPreferencesProvider.notifier)
            .restoreSnapshot(state.prefsSnapshot!);       // Step 3: SKIPPED on Step 2 throw
      }
    }
  } finally {
    controller.setEditMode(false);
    state = const DashboardEditState();
  }
}

The try/finally added in this commit fixes "edit mode won't be stranded" but does NOT fix the data-consistency gap. If saveLayout() throws:

User sees layout visually reverted but prefs remain in post-edit state; on app restart the combination produces inconsistent UI. Trigger: SharedPreferences I/O failure during cancelEditMode (uncommon, not impossible).

Fix: Wrap saveLayout() in its own inner try-catch so restoreSnapshot() always runs:

if (state.layoutSnapshot != null) {
  controller.importLayout(state.layoutSnapshot!);
  try {
    await ref.read(uspSliverDashboardControllerProvider.notifier).saveLayout();
  } catch (e, s) {
    logger.e('[Dashboard] saveLayout failed during revert; continuing to restoreSnapshot',
        error: e, stackTrace: s);
  }
}
if (state.prefsSnapshot != null) {
  await ref.read(uspLayoutPreferencesProvider.notifier)
      .restoreSnapshot(state.prefsSnapshot!);
}

W-2 [both reviewers, CARRY-OVER] · 🟢High — void async view-layer wrappers silently discard exceptions

lib/page/dashboard/views/usp_sliver_dashboard_view.dart:115–121

void _commitEditMode() async {
  await ref.read(dashboardEditModeProvider.notifier).commitEditMode();
}

void _cancelEditMode() async {
  await ref.read(dashboardEditModeProvider.notifier).cancelEditMode();
}

Both remain void async with no try-catch. Exceptions from the notifier are silently swallowed. Compare: route_usp_dashboard.dart's onExit already wraps cancelEditMode() in try-catch with logger.e. The UI-button call sites (✓ commit, ✗ cancel buttons) lack equivalent protection.

Fix (non-breaking):

void _commitEditMode() async {
  try {
    await ref.read(dashboardEditModeProvider.notifier).commitEditMode();
  } catch (e, s) {
    logger.e('[Dashboard] commitEditMode failed', error: e, stackTrace: s);
  }
}
void _cancelEditMode() async {
  try {
    await ref.read(dashboardEditModeProvider.notifier).cancelEditMode();
  } catch (e, s) {
    logger.e('[Dashboard] cancelEditMode failed', error: e, stackTrace: s);
  }
}

N-1 [single, Reviewer A] · 🟡Med — cancelEditMode during enterEditMode's async gap is a silent no-op

lib/page/dashboard/providers/dashboard_edit_mode_provider.dart:47–72

Future<void> enterEditMode() async {
  if (state.isEditing) return;
  state = const DashboardEditState(isEditing: true);  // snapshots still null here

  await ref.read(uspLayoutPreferencesProvider.notifier).initialized;  // async gap

  // If cancelEditMode() fires HERE: snapshots are null, _exitEditMode(revert:true)
  // skips both importLayout and restoreSnapshot — effectively a no-op.
  // This is SAFE because controller.setEditMode(true) hasn't been called yet,
  // but the invariant is undocumented.

  if (!state.isEditing) return;  // bail-out fires correctly, no stuck state
  // ... capture snapshots, setEditMode(true)
}

The if (!state.isEditing) return guard correctly prevents stuck state. The silent no-op is functionally safe (controller not yet in edit mode, no data to revert). Risk is low in practice (window is as long as initialized is pending, typically near-zero on warm start). The TOCTOU test does not exercise a genuinely-pending initialized.

Suggest: Add code comment explaining the null-snapshot invariant so future maintainers understand the silent no-op is intentional.

• PeterJhong's _openLayoutSettings fix confirmed — reset/preset_changed paths now correctly call commitEditMode(), preserving the user's just-applied change. toggle_off dead-code removed correctly (settings panel never emits this string at HEAD — verified).
• Re-entrant guard and TOCTOU fix confirmed — if (state.isEditing) return; + pre-await state claim + post-await bail-out are correct, symmetric, and covered by regression tests.
• finally block guarantees no stuck edit-mode — even if saveLayout() and restoreSnapshot() both fail, setEditMode(false) and state = const DashboardEditState() always execute.
• Route guard error handling — cancelEditMode() in route_usp_dashboard.dart:onExit correctly wrapped in try-catch; navigation never blocked.
• Test suite additions — commitEditMode preserves layout changes (PeterJhong regression), re-entrant guard, TOCTOU cancel-during-async-gap, multi-cycle tests comprehensively cover the corrected behavior.
• Architecture compliance — NotifierProvider (non-autoDispose), all ref.read inside notifier, correct 3-layer boundaries maintained throughout.
• No security issues — no hardcoded secrets, no injection vectors, no permission changes, no JNAP/USP response handling modifications.

W-1 [CARRY-OVER, single, Reviewer A] · 🟢High — saveLayout() exception skips restoreSnapshot()

lib/page/dashboard/providers/dashboard_edit_mode_provider.dart:89–109

Future<void> _exitEditMode({required bool revert}) async {
  final controller = ref.read(uspSliverDashboardControllerProvider);

  try {
    if (revert) {
      if (state.layoutSnapshot != null) {
        controller.importLayout(state.layoutSnapshot!);   // Step 1: in-memory revert
        await ref
            .read(uspSliverDashboardControllerProvider.notifier)
            .saveLayout();                                // Step 2: can throw
      }
      if (state.prefsSnapshot != null) {
        await ref
            .read(uspLayoutPreferencesProvider.notifier)
            .restoreSnapshot(state.prefsSnapshot!);       // Step 3: SKIPPED if Step 2 throws
      }
    }
  } finally {
    controller.setEditMode(false);
    state = const DashboardEditState();                   // stuck-mode fixed — consistency gap remains
  }
}

The Round 3 try/finally correctly fixed stuck edit-mode. But the data-consistency gap is unchanged: if saveLayout() throws, control jumps to finally, skipping restoreSnapshot(). After failure:

User sees grid visually reverted but prefs remain mid-edit; on restart the combination produces inconsistent UI. Trigger: SharedPreferences I/O failure during cancelEditMode.

Fix: Wrap saveLayout() in its own inner try-catch so restoreSnapshot() always executes:

if (state.layoutSnapshot != null) {
  controller.importLayout(state.layoutSnapshot!);
  try {
    await ref.read(uspSliverDashboardControllerProvider.notifier).saveLayout();
  } catch (e, s) {
    logger.e('[Dashboard] saveLayout failed during revert; continuing to restoreSnapshot',
        error: e, stackTrace: s);
  }
}
if (state.prefsSnapshot != null) {
  await ref.read(uspLayoutPreferencesProvider.notifier)
      .restoreSnapshot(state.prefsSnapshot!);
}

W-2 [CARRY-OVER, both reviewers] · 🟢High — void async view wrappers silently discard exceptions from button taps

lib/page/dashboard/views/usp_sliver_dashboard_view.dart:115–119

void _commitEditMode() async {
  await ref.read(dashboardEditModeProvider.notifier).commitEditMode();
}

void _cancelEditMode() async {
  await ref.read(dashboardEditModeProvider.notifier).cancelEditMode();
}

Both remain void async with no try-catch. The route_usp_dashboard.dart:onExit path now has a try-catch (good), but the check/close button paths (onTap: _commitEditMode, onTap: _cancelEditMode) do not. If saveLayout() throws from a button tap, the user sees no feedback and no error is logged from the view layer.

Fix:

void _commitEditMode() async {
  try {
    await ref.read(dashboardEditModeProvider.notifier).commitEditMode();
  } catch (e, s) {
    logger.e('[Dashboard] commitEditMode failed', error: e, stackTrace: s);
  }
}
void _cancelEditMode() async {
  try {
    await ref.read(dashboardEditModeProvider.notifier).cancelEditMode();
  } catch (e, s) {
    logger.e('[Dashboard] cancelEditMode failed', error: e, stackTrace: s);
  }
}

W-3 [NEW, both reviewers] · 🟢High — _exitEditMode lacks isEditing idempotency guard

lib/page/dashboard/providers/dashboard_edit_mode_provider.dart:89

enterEditMode() has if (state.isEditing) return; but _exitEditMode has no symmetric guard. If both _cancelEditMode (button) and onExit (route) fire concurrently in an async gap:

1. First call starts _exitEditMode(revert: true), importLayout runs, saveLayout() awaiting.
2. Second call enters _exitEditMode(revert: true) — state.layoutSnapshot is null (first call's finally already cleared state), so importLayout/restoreSnapshot skipped, but controller.setEditMode(false) and state = const DashboardEditState() fire again — triggering unnecessary Riverpod listener notifications.

While the finally guarantees correct end state, the duplicate controller/state updates are observable side-effects.

Fix:

Future<void> _exitEditMode({required bool revert}) async {
  if (!state.isEditing) return;  // symmetric guard matching enterEditMode
  final controller = ref.read(uspSliverDashboardControllerProvider);
  // ... rest unchanged
}

W-4 [NEW, single, Reviewer B] · 🟢High — Missing isEditing==false idempotency unit tests

test/page/dashboard/providers/dashboard_edit_mode_provider_test.dart

This round added four valuable tests (re-entrant guard, async-gap cancel, PeterJhong regression, multi-cycle). However, none directly tests calling commitEditMode() or cancelEditMode() when isEditing == false. If W-3's guard is added, this path becomes a documented early-return; without a test, future refactors could break it silently.

Fix: Add:

test('commitEditMode when not editing is a safe no-op', () async {
  final container = await createContainer();
  await container.read(dashboardEditModeProvider.notifier).commitEditMode();
  expect(container.read(dashboardEditModeProvider).isEditing, isFalse);
});

test('cancelEditMode when not editing is a safe no-op', () async {
  final container = await createContainer();
  await container.read(dashboardEditModeProvider.notifier).cancelEditMode();
  expect(container.read(dashboardEditModeProvider).isEditing, isFalse);
});

• PeterJhong's _openLayoutSettings fix confirmed — reset/preset_changed now correctly call commitEditMode() (keeps the applied change). toggle_off dead code removed cleanly.
• Re-entrant enterEditMode guard — if (state.isEditing) return; prevents snapshot overwrite on double-tap; state claimed before await ensures onExit always observes edit state.
• Async-gap cancel covered — if (!state.isEditing) return; post-await bail-out correct, now has a dedicated test.
• finally block guarantees no stuck edit-mode — setEditMode(false) and state = DashboardEditState() always execute regardless of I/O failures.
• linksys_route.dart deleted — Duplicate LinksysRoute subclass using deprecated FeatureState.isDirty API removed; route_model.dart is now the single definition.
• route_usp_dashboard.dart onExit hardening — cancelEditMode() wrapped in try-catch with logger.e; navigation never blocked on revert failure.
• layoutSnapshot type narrowed — List<dynamic>? to List<Map<String,dynamic>>? correctly reflects DashboardController.exportLayout() return type.
• API rename (commit/cancel) — Clearer semantics than save: bool parameter; all call sites updated.
• Architecture compliance — NotifierProvider (non-autoDispose), all ref.read inside notifier, 3-layer boundaries maintained.
• Test suite additions — Re-entrant guard, TOCTOU cancel-during-gap, PeterJhong regression, multi-cycle tests comprehensively cover the corrected behavior.
• No security issues — No hardcoded secrets, no injection vectors, no JNAP/USP response handling modifications.

W-1 [both reviewers, CARRY-OVER] · High — saveLayout() exception skips restoreSnapshot()

lib/page/dashboard/providers/dashboard_edit_mode_provider.dart:94-103

try {
  if (revert) {
    if (state.layoutSnapshot != null) {
      controller.importLayout(state.layoutSnapshot!);
      await ref
          .read(uspSliverDashboardControllerProvider.notifier)
          .saveLayout();  // can throw; restoreSnapshot SKIPPED if this throws
    }
    if (state.prefsSnapshot != null) {
      await ref
          .read(uspLayoutPreferencesProvider.notifier)
          .restoreSnapshot(state.prefsSnapshot!);  // never reached on saveLayout throw
    }
  }
} finally {
  controller.setEditMode(false);
  state = const DashboardEditState();  // snapshots wiped; no retry possible
}

When saveLayout() throws (SharedPreferences I/O failure), finally fires clearing the edit flag and snapshots, but restoreSnapshot never ran. Post-failure: grid in-memory = reverted, grid on-disk = still edited, prefs = still edited. Three sources of truth are inconsistent and unrecoverable.

Fix: Wrap each revert step independently:

if (state.layoutSnapshot != null) {
  controller.importLayout(state.layoutSnapshot!);
  try {
    await ref.read(uspSliverDashboardControllerProvider.notifier).saveLayout();
  } catch (e, s) {
    logger.e('[Dashboard] saveLayout failed during revert — continuing to restoreSnapshot',
        error: e, stackTrace: s);
  }
}
if (state.prefsSnapshot != null) {
  await ref.read(uspLayoutPreferencesProvider.notifier)
      .restoreSnapshot(state.prefsSnapshot!);
}

W-2 [both reviewers, CARRY-OVER] · High — void async view wrappers silently discard exceptions

lib/page/dashboard/views/usp_sliver_dashboard_view.dart:115-122

void _commitEditMode() async {
  await ref.read(dashboardEditModeProvider.notifier).commitEditMode();
}

void _cancelEditMode() async {
  await ref.read(dashboardEditModeProvider.notifier).cancelEditMode();
}

Both remain void async with no try-catch. Any exception propagates into an unhandled Future and is silently swallowed. The route onExit path was correctly wrapped in try-catch this round; the button-tap paths were not.

Fix:

void _commitEditMode() async {
  try {
    await ref.read(dashboardEditModeProvider.notifier).commitEditMode();
  } catch (e, s) {
    logger.e('[Dashboard]: commitEditMode failed', error: e, stackTrace: s);
  }
}
void _cancelEditMode() async {
  try {
    await ref.read(dashboardEditModeProvider.notifier).cancelEditMode();
  } catch (e, s) {
    logger.e('[Dashboard]: cancelEditMode failed', error: e, stackTrace: s);
  }
}

W-3 [both reviewers, CARRY-OVER] · High — _exitEditMode lacks isEditing guard

lib/page/dashboard/providers/dashboard_edit_mode_provider.dart:89

enterEditMode() has if (state.isEditing) return; but _exitEditMode has no symmetric guard. Concurrent path: user taps commit then cancel before saveLayout() returns. Since state.isEditing is still true during the await, the second call enters _exitEditMode(revert: true) and starts a full revert in parallel. Both race to call controller.setEditMode(false) and state = const DashboardEditState(). Outcome: commit followed by an unintended revert. The route onExit is guarded (if (editState.isEditing)); the provider API is not.

Fix:

Future<void> _exitEditMode({required bool revert}) async {
  if (!state.isEditing) return;  // symmetric guard matching enterEditMode
  final controller = ref.read(uspSliverDashboardControllerProvider);
  // ... rest unchanged
}

W-4 [both reviewers, CARRY-OVER] · Med — Missing isEditing==false idempotency tests

test/page/dashboard/providers/dashboard_edit_mode_provider_test.dart

New tests cover re-entrant guard, async-gap cancel, PeterJhong regression, multi-cycle. None test calling commitEditMode() or cancelEditMode() from an isEditing == false baseline.

Fix:

test('commitEditMode when not editing is a safe no-op', () async {
  final container = await createContainer();
  await container.read(dashboardEditModeProvider.notifier).commitEditMode();
  expect(container.read(dashboardEditModeProvider).isEditing, isFalse);
});
test('cancelEditMode when not editing is a safe no-op', () async {
  final container = await createContainer();
  await container.read(dashboardEditModeProvider.notifier).cancelEditMode();
  expect(container.read(dashboardEditModeProvider).isEditing, isFalse);
});

W-5 [both reviewers, NEW] · High — toggle_off result silently dropped without documentation

lib/page/dashboard/views/usp_sliver_dashboard_view.dart:514

- if ((result == 'reset' || result == 'toggle_off' || result == 'preset_changed') && mounted) {
+ if ((result == 'reset' || result == 'preset_changed') && mounted) {

Both reviewers verified that usp_layout_settings_panel.dart currently does not emit toggle_off — so the removal is functionally correct today. However, project documentation lists toggle_off as a valid dialog result. With no comment explaining the removal, a future contributor who re-adds Navigator.pop(context, 'toggle_off') will get a silent no-op: no commit, no cancel — edit mode stays active after the dialog closes.

Fix:

// Note: 'toggle_off' intentionally removed — settings panel no longer emits this
// result as of this PR. If re-added to the panel, update this handler accordingly.
if ((result == 'reset' || result == 'preset_changed') && mounted) {

• PeterJhong's _openLayoutSettings regression RESOLVED — reset and preset_changed now correctly call commitEditMode() (preserve the applied change). Prior exitEditMode(save: false) that inadvertently reverted the user's just-applied change is gone. Regression test added.
• Re-entrant enterEditMode guard confirmed — if (state.isEditing) return; and pre-await state claim correctly prevent snapshot overwrite and handle route-guard async gap.
• finally guarantees no stuck edit-mode — setEditMode(false) and state = DashboardEditState() always execute regardless of I/O failures.
• route_usp_dashboard.dart onExit hardened — cancelEditMode() wrapped in try-catch with logger; navigation never blocked on revert failure.
• linksys_route.dart deleted — Duplicate LinksysRoute using deprecated FeatureState.isDirty API cleanly removed.
• layoutSnapshot type narrowed — List<dynamic>? to List<Map<String,dynamic>>? correctly reflects exportLayout() return type.
• API rename (commit/cancel) — Clearer semantics than save: bool parameter; all call sites updated consistently.
• Test suite additions — Re-entrant guard, async-gap cancel, PeterJhong regression, multi-cycle tests comprehensively cover the corrected behavior.
• Architecture compliance — NotifierProvider (non-autoDispose), all ref.read inside notifier, 3-layer boundaries maintained.
• No security issues — No hardcoded secrets, no injection vectors, no JNAP/USP response handling modifications.

• PeterJhong regression RESOLVED — _openLayoutSettings now calls commitEditMode() instead of exitEditMode(save: false), correctly preserving reset/preset changes. Regression test added.
• W-5 RESOLVED — toggle_off removal contextualised; switching handler to commitEditMode() makes absence logical.
• enterEditMode re-entrant guard RESOLVED — if (state.isEditing) return; + early isEditing: true claim + post-await bail-out correctly handles double-tap race and navigation-during-init async gap.

W-1 [both reviewers, CARRY-OVER] High — saveLayout() exception skips restoreSnapshot()

lib/page/dashboard/providers/dashboard_edit_mode_provider.dart:92-108

try {
  if (revert) {
    if (state.layoutSnapshot != null) {
      controller.importLayout(state.layoutSnapshot!);
      await ref.read(uspSliverDashboardControllerProvider.notifier).saveLayout();
      // throws here → restoreSnapshot SKIPPED below
    }
    if (state.prefsSnapshot != null) {
      await ref.read(uspLayoutPreferencesProvider.notifier).restoreSnapshot(state.prefsSnapshot!);
    }
  }
} finally {
  controller.setEditMode(false);
  state = const DashboardEditState();  // snapshots wiped; no retry possible
}

finally correctly prevents stuck edit mode. But if saveLayout() throws (SharedPreferences I/O failure), restoreSnapshot() is skipped. Post-failure: grid in-memory = reverted to snapshot, grid on-disk = still edited, prefs = still reflect edit-mode values. Three subsystems left inconsistent; snapshots wiped in finally so no retry possible.

Fix: Separate try/catch per awaitable so restoreSnapshot always runs:

if (state.layoutSnapshot != null) {
  controller.importLayout(state.layoutSnapshot!);
  try {
    await ref.read(uspSliverDashboardControllerProvider.notifier).saveLayout();
  } catch (e, s) {
    logger.e('[Dashboard] saveLayout failed during revert', error: e, stackTrace: s);
  }
}
if (state.prefsSnapshot != null) {
  await ref.read(uspLayoutPreferencesProvider.notifier).restoreSnapshot(state.prefsSnapshot!);
}

W-2 [both reviewers, CARRY-OVER] High — _commitEditMode/_cancelEditMode remain void async

lib/page/dashboard/views/usp_sliver_dashboard_view.dart:115-121

void _commitEditMode() async {
  await ref.read(dashboardEditModeProvider.notifier).commitEditMode();
}

void _cancelEditMode() async {
  await ref.read(dashboardEditModeProvider.notifier).cancelEditMode();
}

Both remain void async without try/catch. Any exception propagates into an unhandled Future and is silently swallowed. Route onExit correctly wraps cancelEditMode() in try/catch; button-tap paths do not.

Fix: Add try/catch with logger.e to both wrappers (same pattern as route_usp_dashboard.dart).

W-3 [both reviewers, CARRY-OVER] High — _exitEditMode lacks isEditing guard

lib/page/dashboard/providers/dashboard_edit_mode_provider.dart:89

enterEditMode() has the correct re-entrant guard. _exitEditMode still has no symmetric guard. Concurrent path: user taps commit while route onExit fires simultaneously. Both enter _exitEditMode; the second call reaches controller.setEditMode(false) a second time and creates redundant state churn.

Fix:

Future<void> _exitEditMode({required bool revert}) async {
  if (!state.isEditing) return;  // symmetric guard matching enterEditMode
  // ... rest unchanged
}

W-4 [single, Reviewer A, CARRY-OVER] Med — Missing isEditing==false idempotency tests

test/page/dashboard/providers/dashboard_edit_mode_provider_test.dart

Tests added this round cover: PeterJhong regression, re-entrant enter guard, async-gap cancel, multi-cycle. None test calling commitEditMode() or cancelEditMode() when isEditing == false. (Reviewer A read the test file directly and confirmed absence; Reviewer B marked resolved without access to the PR branch.)

Fix: Add two tests verifying safe no-op behavior when not in edit mode.

W-7 [single, Reviewer A, NEW] High — Widget added via settings panel then Cancel reverts it

lib/page/dashboard/views/usp_sliver_dashboard_view.dart:514-519 + usp_layout_settings_panel.dart

if ((result == 'reset' || result == 'preset_changed') && mounted) {
  await ref.read(dashboardEditModeProvider.notifier).commitEditMode();
}

When the settings panel Close button pops with no argument, result == null. Condition is false; commitEditMode() not called. Inside the panel, addWidget() calls saveLayout() immediately — widget is persisted to disk. But snapshot captured in enterEditMode() does not include the new widget. If user then taps Cancel, cancelEditMode() restores the snapshot — widget removed from memory and disk despite having been saved.

Concrete trigger: Edit mode → Settings panel → "+" add widget → Close (no result) → tap Cancel → widget disappears.

Fix (Option 1): Pop with 'widget_added' after addWidget() in the settings panel; add that string to the condition in _openLayoutSettings.

Fix (Option 2): Add refreshSnapshot() to the notifier and call it after widget mutations inside the settings panel.

• PeterJhong regression correctly fixed — commitEditMode() preserves reset/preset changes. Regression test added.
• enterEditMode re-entrant guard — correct order: guard, early state claim, await, post-await bail-out.
• try/finally guarantees no stuck edit mode — setEditMode(false) and state always cleared regardless of I/O failures.
• Route onExit error handling — try/catch with structured logging; navigation never blocked.
• Type narrowing — List<dynamic> to List<Map<String, dynamic>> surfaces type errors at compile time.
• linksys_route.dart deletion — dead file using deprecated FeatureState.isDirty correctly removed.
• New tests — PeterJhong regression, re-entrant guard, async-gap cancel, multi-cycle tests well-targeted.
• No security issues — no hardcoded secrets, injection vectors, or JNAP/USP response handling modifications.
• Architecture compliance — NotifierProvider (non-autoDispose), ref.read inside notifier, 3-layer boundaries maintained.

• PeterJhong regression RESOLVED — _openLayoutSettings (line 514) now calls commitEditMode() for reset and preset_changed results, correctly keeping the applied change instead of reverting to pre-edit snapshot. Regression test added.
• toggle_off removal confirmed safe — global search finds zero remaining toggle_off strings in .dart files; no matching pop calls exist.
• linksys_route.dart deletion confirmed safe — LinksysRoute class was already migrated to route_model.dart using PreservableContract; no dangling imports found.
• DashboardEditState type narrowing — layoutSnapshot now typed List<Map<String, dynamic>>? (narrowed from List<dynamic>?) and copyWith updated consistently.
• enterEditMode re-entrant guard + async-gap bail-out — if (state.isEditing) return at line 50 plus if (!state.isEditing) return post-await at line 60 correctly handle double-tap race and navigation-during-init.

W-1 [both reviewers, CARRY-OVER] 🟢High — saveLayout() exception skips restoreSnapshot()

lib/page/dashboard/providers/dashboard_edit_mode_provider.dart:93-104

try {
  if (revert) {
    if (state.layoutSnapshot != null) {
      controller.importLayout(state.layoutSnapshot!);
      await ref.read(uspSliverDashboardControllerProvider.notifier).saveLayout();
      // if this throws, restoreSnapshot() below is NEVER reached
    }
    if (state.prefsSnapshot != null) {
      await ref.read(uspLayoutPreferencesProvider.notifier).restoreSnapshot(state.prefsSnapshot!);
    }
  }
} finally {
  controller.setEditMode(false);
  state = const DashboardEditState(); // snapshots wiped; no retry possible
}

The finally block correctly prevents stuck edit mode. However, if saveLayout() throws (SharedPreferences/localStorage I/O failure), restoreSnapshot() is skipped. Post-failure: grid in-memory is reverted to snapshot, grid on-disk still reflects edited state, prefs still reflect edit-mode values. Three subsystems are now inconsistent; snapshots wiped in finally so no retry is possible.

Fix: Give each awaitable its own try/catch so restoreSnapshot always runs:

if (revert) {
  if (state.layoutSnapshot != null) {
    controller.importLayout(state.layoutSnapshot!);
    try {
      await ref.read(uspSliverDashboardControllerProvider.notifier).saveLayout();
    } catch (e, s) {
      logger.e('[Dashboard] saveLayout failed during cancel', error: e, stackTrace: s);
    }
  }
  if (state.prefsSnapshot != null) {
    await ref.read(uspLayoutPreferencesProvider.notifier).restoreSnapshot(state.prefsSnapshot!);
  }
}

W-2 [both reviewers, CARRY-OVER] 🟢High — _commitEditMode/_cancelEditMode remain void async

lib/page/dashboard/views/usp_sliver_dashboard_view.dart:115-121

void _commitEditMode() async {
  await ref.read(dashboardEditModeProvider.notifier).commitEditMode();
}

void _cancelEditMode() async {
  await ref.read(dashboardEditModeProvider.notifier).cancelEditMode();
}

Both widget-layer wrappers are void async without try/catch. Any exception from the provider is silently discarded. The route's onExit wrapper (route_usp_dashboard.dart:35-44) correctly catches and logs — button-tap paths do not.

Fix: Add try/catch with logger.e (matching route pattern):

void _commitEditMode() async {
  try {
    await ref.read(dashboardEditModeProvider.notifier).commitEditMode();
  } catch (e, s) {
    logger.e('[Dashboard] commitEditMode failed', error: e, stackTrace: s);
  }
}

W-3 [both reviewers, CARRY-OVER] 🟢High — _exitEditMode lacks isEditing guard

lib/page/dashboard/providers/dashboard_edit_mode_provider.dart:89

enterEditMode() has a re-entrant guard (if (state.isEditing) return). _exitEditMode has no symmetric guard. Double-call scenario: user taps Cancel button while route onExit fires simultaneously. Both calls enter _exitEditMode with state.isEditing == true; both execute importLayout() + saveLayout() in parallel, racing on the same storage key. Second call also sees non-null prefsSnapshot (first call's finally hasn't run yet) and triggers a second restoreSnapshot.

Fix:

Future<void> _exitEditMode({required bool revert}) async {
  if (!state.isEditing) return; // symmetric guard matching enterEditMode
  // ... rest unchanged
}

W-4 [both reviewers, CARRY-OVER] 🟡Med — Missing isEditing==false idempotency tests

test/page/dashboard/providers/dashboard_edit_mode_provider_test.dart

New tests this round cover re-entrant enter, async-gap cancel, and commit-preserves-changes. None test calling commitEditMode() or cancelEditMode() when isEditing == false — the no-op/idempotency behavior when not in edit mode is not verified.

Fix: Add two tests:

test('commitEditMode when not editing is a no-op', () async {
  final container = await createContainer();
  await container.read(dashboardEditModeProvider.notifier).commitEditMode();
  expect(container.read(dashboardEditModeProvider).isEditing, isFalse);
});
test('cancelEditMode when not editing is a no-op', () async {
  final container = await createContainer();
  await container.read(dashboardEditModeProvider.notifier).cancelEditMode();
  expect(container.read(dashboardEditModeProvider).isEditing, isFalse);
});

W-7 [single — CONTESTED] 🟡Med — Widget added via settings panel then Cancel

lib/page/dashboard/views/usp_sliver_dashboard_view.dart:514-518

if ((result == 'reset' || result == 'preset_changed') && mounted) {
  await ref.read(dashboardEditModeProvider.notifier).commitEditMode();
}
// When user adds a widget (+) then closes the panel (result == null):
// commitEditMode() is NOT called; if user taps Cancel → widget reverted

Reviewer A: Data-loss edge case. The user adds a widget (persisted to disk by addWidget()), closes the panel without triggering 'reset'/'preset_changed', then taps Cancel — the widget disappears despite having been explicitly added.

Reviewer B: Cancel is designed to revert all edit-mode changes to the pre-edit snapshot. A widget added during edit mode reverting on Cancel is correct semantics; the snapshot accurately reflects the pre-edit state.

Synthesizer note: Austin to decide — should widgets added via the settings panel be treated as committed sub-actions that survive a subsequent Cancel? If yes, add a refreshSnapshot() call after addWidget(). If Cancel should always restore the pre-edit baseline, current behavior is correct.

• _exitEditMode finally block — finally guarantees controller.setEditMode(false) + state reset regardless of exceptions; dashboard cannot be stranded in edit mode.
• enterEditMode double-guard pattern (lines 50 + 60) — claim isEditing: true before the first await; bail out post-await if cancelled. Both guards are correct and well-commented.
• commitEditMode/cancelEditMode semantic rename — eliminates the save: false inverted-boolean trap; API intent is now unambiguous.
• PeterJhong regression fix — commitEditMode() used correctly for reset/preset_changed path; toggle_off dead branch cleanly removed.
• layoutSnapshot type narrowing — List<dynamic>? → List<Map<String, dynamic>>? reduces implicit cast risk.
• route_usp_dashboard.dart onExit try/catch — catches and logs without blocking navigation; correct strategy.
• New regression test suite — re-entrant enter, async-gap cancel, and commit-preserves-changes tests are high-quality behavioral regression guards.
• linksys_route.dart deletion — dead code removed cleanly.
• Security — no hardcoded secrets, no injection vectors, no permission changes, no JNAP/USP response handling modifications.

• PeterJhong regression RESOLVED — _openLayoutSettings (line 514) now calls commitEditMode() for reset and preset_changed results, correctly keeping the applied change instead of reverting to pre-edit snapshot. toggle_off branch removed (confirmed dead — settings panel never pops this value).
• W-1 controller cleanup RESOLVED — _exitEditMode wraps work in try/finally; controller.setEditMode(false) and state = const DashboardEditState() are now guaranteed to execute even when saveLayout() or restoreSnapshot() throws. Dashboard can no longer be stranded in edit mode on I/O failure.
• Re-entrant enterEditMode RESOLVED — if (state.isEditing) return guard at line 50 prevents double-tap/gesture race from overwriting the original snapshot. Post-await if (!state.isEditing) return bail-out correctly handles cancellation during the initialized await.
• DashboardEditState.layoutSnapshot type tightened — narrowed from List<dynamic>? to List<Map<String, dynamic>>?. Verified that sliver_dashboard 0.9.1 declares List<Map<String, dynamic>> exportLayout() — this narrowing is safe and compiles cleanly.
• New regression tests added — commitEditMode-preserves-changes (PeterJhong regression guard), re-entrant-enter (W-1 guard), cancel-during-async-gap (W-2 guard).
• linksys_route.dart deletion confirmed safe — LinksysRoute active implementation lives in route_model.dart; zero imports of the deleted file across the codebase.

W-1 [both reviewers, CARRY-OVER] 🟢High — saveLayout() throw still skips restoreSnapshot()

lib/page/dashboard/providers/dashboard_edit_mode_provider.dart:93-104

try {
  if (revert) {
    if (state.layoutSnapshot != null) {
      controller.importLayout(state.layoutSnapshot!);
      await ref.read(...).saveLayout();         // ← if THIS throws …
    }
    if (state.prefsSnapshot != null) {
      await ref.read(...).restoreSnapshot(...); // ← … STILL skipped
    }
  }
} finally {
  controller.setEditMode(false);   // ✅ now guaranteed
  state = const DashboardEditState(); // ✅ now guaranteed
}

The finally block correctly prevents stuck edit mode (improvement). But if saveLayout() throws (SharedPreferences/localStorage I/O failure), restoreSnapshot() is skipped. Post-failure: grid in-memory is reverted to snapshot, grid on-disk still reflects edited state, prefs (selectedPreset) still reflect the in-progress edit. Three subsystems are inconsistent; snapshots wiped in finally so no retry possible.

Fix: Give each awaitable its own try/catch so restoreSnapshot always runs regardless of saveLayout success:

if (state.layoutSnapshot != null) {
  controller.importLayout(state.layoutSnapshot!);
  try {
    await ref.read(uspSliverDashboardControllerProvider.notifier).saveLayout();
  } catch (e, s) {
    logger.e('[Dashboard] saveLayout failed during cancel', error: e, stackTrace: s);
  }
}
if (state.prefsSnapshot != null) {
  await ref.read(uspLayoutPreferencesProvider.notifier).restoreSnapshot(state.prefsSnapshot!);
}

W-2 [both reviewers, CARRY-OVER] 🟢High — _commitEditMode/_cancelEditMode bare void async

lib/page/dashboard/views/usp_sliver_dashboard_view.dart:115-121

void _commitEditMode() async {
  await ref.read(dashboardEditModeProvider.notifier).commitEditMode(); // no try/catch
}
void _cancelEditMode() async {
  await ref.read(dashboardEditModeProvider.notifier).cancelEditMode(); // no try/catch
}

The provider's finally block prevents state corruption, but a saveLayout() I/O failure during revert will propagate out through cancelEditMode() → _cancelEditMode() → silently consumed by Flutter's unhandled-async zone. The route guard (same commit) correctly wraps the same call with try/catch + logger.e. The two call-sites have inconsistent error-handling discipline.

Fix: Add try/catch matching the route guard pattern:

void _cancelEditMode() async {
  try {
    await ref.read(dashboardEditModeProvider.notifier).cancelEditMode();
  } catch (e, s) {
    logger.e('[Dashboard] cancelEditMode failed', error: e, stackTrace: s);
  }
}

W-3 [both reviewers, CARRY-OVER] 🟢High — _exitEditMode lacks isEditing guard

lib/page/dashboard/providers/dashboard_edit_mode_provider.dart:89

enterEditMode() now has a symmetric re-entrant guard (if (state.isEditing) return). _exitEditMode has no matching if (!state.isEditing) return.

Double-exit scenario: user taps ✗ Cancel while route onExit fires simultaneously. Both calls enter _exitEditMode(revert: true) with state.isEditing == true. Both read state.layoutSnapshot (non-null — first call's finally has not yet run), both execute importLayout() + saveLayout() concurrently, racing on the same SharedPreferences key.

Fix:

Future<void> _exitEditMode({required bool revert}) async {
  if (!state.isEditing) return; // symmetric guard matching enterEditMode
  // ... rest unchanged
}

W-4 [both reviewers, CARRY-OVER] 🟡Med — Missing idempotency tests

test/page/dashboard/providers/dashboard_edit_mode_provider_test.dart

New tests cover re-entrant enter, async-gap cancel, and commit-preserves-changes. None verify that calling commitEditMode() or cancelEditMode() when isEditing == false is a no-op (or at minimum doesn't corrupt state). Without such a test, the W-3 double-call scenario and the controller.setEditMode(false) unconditional call are invisible to CI.

Fix: Add two tests:

test('commitEditMode when not editing is a no-op', () async { ... });
test('cancelEditMode when not editing is a no-op', () async { ... });

ARCH-04 [single — architecture] 🟡Med — _openLayoutSettings exits edit mode on reset/preset_changed

lib/page/dashboard/views/usp_sliver_dashboard_view.dart:514

if ((result == 'reset' || result == 'preset_changed') && mounted) {
  // Commit (keep) the change — the settings panel already applied the
  // reset/preset directly to the controller and prefs …
  await ref.read(dashboardEditModeProvider.notifier).commitEditMode();
}

commitEditMode() exits edit mode entirely. A user who opens Settings → selects a new preset → closes the panel is silently returned to the non-editing dashboard state, instead of back to edit mode. The bug that PeterJhong flagged (changes were reverted) is fixed, but the flow now ejects the user from edit mode without indication. This is a second-order UX regression — consider returning null from the panel close to keep edit mode alive, rather than terminating it, unless fully exiting is the intended design.

• No security issues — no hardcoded secrets, no injection vectors, no JNAP/USP response handling touched.
• finally block is correct — state always resets, edit mode never gets stranded on I/O failure.
• Re-entrant enterEditMode guard — if (state.isEditing) return + pre-await isEditing: true claim + post-await bail-out is the right pattern for this async lifecycle.
• commitEditMode()/cancelEditMode() naming — explicit names are clearer than exitEditMode({bool save}) which was an inverted-boolean trap.
• Route onExit hardened correctly — try/catch + logger.e + always returns true (never blocks navigation) is the correct pattern.
• Test coverage for the regression — commitEditMode-preserves-changes directly guards against the PeterJhong regression reoccurring.
• layoutSnapshot type tightening — List<Map<String, dynamic>>? is correct; verified against sliver_dashboard 0.9.1 which declares List<Map<String, dynamic>> exportLayout().

• W-1 [both reviewers] RESOLVED — _exitEditMode now wraps both revert operations in try/finally. controller.setEditMode(false) and state = DashboardEditState() are guaranteed to run even if saveLayout() or restoreSnapshot() throws — dashboard can no longer be stranded in edit mode on I/O failure. (Residual: W-1 carry-over re-emerges as N-1 below — the finally solves "stuck in edit mode" but not "prefs diverge from grid".)
• W-2 [both reviewers] RESOLVED — State integrity is now owned by the provider layer (try/finally). The view's _commitEditMode/_cancelEditMode correctly delegate; provider guarantees state cleanup. Route-level onExit has its own try/catch + logger.e for the high-stakes path. (New logging gap remains — see N-2.)
• W-3 [both reviewers] RESOLVED — enterEditMode() has re-entrant guard at line 50 (if (state.isEditing) return) and post-await bail-out at line 60 (if (!state.isEditing) return). Both synchronous double-tap race and navigation-during-async-gap race are covered. Calling _exitEditMode when isEditing == false is safe (revert path short-circuits on null snapshots; finally writes are idempotent). (Spurious rebuild concern noted as N-3.)
• ARCH-04 [single — architecture] RESOLVED — _openLayoutSettings now calls commitEditMode() (keep) instead of exitEditMode(save: false) (revert) for reset/preset_changed results. toggle_off branch correctly removed (confirmed no hit in codebase — was always a dead path). PeterJhong's regression is fixed.
• Re-entrant enterEditMode RESOLVED — Pre-await isEditing: true claim + post-await bail-out correctly handles both the double-tap race and the navigate-during-init-gap scenario.
• layoutSnapshot type tightened — Narrowed from List<dynamic>? to List<Map<String, dynamic>>?; consistent with sliver_dashboard 0.9.1's exportLayout() return type.
• PeterJhong regression fully addressed — commitEditMode-preserves-changes test directly guards against recurrence.

N-1 [both reviewers] 🟢High — Sequential revert steps share one try; saveLayout() throw still skips restoreSnapshot()

lib/page/dashboard/providers/dashboard_edit_mode_provider.dart:92–104

try {
  if (revert) {
    if (state.layoutSnapshot != null) {
      controller.importLayout(state.layoutSnapshot!);
      await ref.read(uspSliverDashboardControllerProvider.notifier).saveLayout(); // ← if this throws…
    }
    if (state.prefsSnapshot != null) {
      await ref.read(uspLayoutPreferencesProvider.notifier).restoreSnapshot(state.prefsSnapshot!); // ← …skipped
    }
  }
} finally {
  controller.setEditMode(false); // ✅ always runs
  state = const DashboardEditState(); // ✅ always runs
}

The finally correctly prevents stuck edit mode. But when saveLayout() throws (SharedPreferences/localStorage I/O failure during cancel), restoreSnapshot() is never reached. Post-failure: grid is reverted in-memory, grid on-disk still reflects edited state, prefs (selectedPreset) still reflect the in-progress edit — three subsystems inconsistent. Snapshots wiped in finally so no retry possible.

Trigger: SharedPreferences I/O failure (quota exceeded, corrupted storage, race with another write) during cancel-edit-mode on Android/iOS.

Fix: Wrap each awaitable independently:

if (state.layoutSnapshot != null) {
  controller.importLayout(state.layoutSnapshot!);
  try {
    await ref.read(uspSliverDashboardControllerProvider.notifier).saveLayout();
  } catch (e, s) {
    logger.e('[Dashboard] saveLayout failed during revert', error: e, stackTrace: s);
  }
}
if (state.prefsSnapshot != null) {
  await ref.read(uspLayoutPreferencesProvider.notifier).restoreSnapshot(state.prefsSnapshot!);
}

N-2 [both reviewers] 🟢High — _commitEditMode/_cancelEditMode bare void async with no error logging

lib/page/dashboard/views/usp_sliver_dashboard_view.dart:115–121

void _commitEditMode() async {
  await ref.read(dashboardEditModeProvider.notifier).commitEditMode(); // no try/catch
}
void _cancelEditMode() async {
  await ref.read(dashboardEditModeProvider.notifier).cancelEditMode(); // no try/catch
}

Provider's finally prevents state corruption but any exception escaping commitEditMode()/cancelEditMode() propagates into the void async frame and is silently consumed by Flutter's unhandled-async zone — no log, no user feedback. The route guard (route_usp_dashboard.dart:27–35) has try/catch + logger.e; the two button-tap call sites have inconsistent error-handling discipline.

Fix: Add try/catch matching the route guard pattern:

void _cancelEditMode() async {
  try {
    await ref.read(dashboardEditModeProvider.notifier).cancelEditMode();
  } catch (e, s) {
    logger.e('[Dashboard] cancelEditMode failed', error: e, stackTrace: s);
  }
}
void _commitEditMode() async {
  try {
    await ref.read(dashboardEditModeProvider.notifier).commitEditMode();
  } catch (e, s) {
    logger.e('[Dashboard] commitEditMode failed', error: e, stackTrace: s);
  }
}

N-3 [single — architecture] 🟡Med — _exitEditMode lacks isEditing guard; spurious Riverpod rebuilds on every tab-leave

lib/page/dashboard/providers/dashboard_edit_mode_provider.dart:89

enterEditMode() has a symmetric re-entrant guard (if (state.isEditing) return). _exitEditMode has no matching if (!state.isEditing) return. When called outside edit mode (e.g., if a route guard fires on a tab where the user never entered edit mode), the finally block unconditionally calls controller.setEditMode(false) and assigns state = const DashboardEditState() — triggering Riverpod listener rebuilds even when nothing changed.

Fix:

Future<void> _exitEditMode({required bool revert}) async {
  if (!state.isEditing) return; // symmetric guard matching enterEditMode
  // ... rest unchanged
}

Note: route_usp_dashboard.dart:34 already gates if (editState.isEditing) before calling cancelEditMode(), so the route path is safe. The guard in the provider itself would protect future callers.

N-4 [single — security] 🟡Med — Snapshot is null during await initialized async gap; cancel during gap skips layout revert

lib/page/dashboard/providers/dashboard_edit_mode_provider.dart:54–60

// Line 54: isEditing = true, layoutSnapshot = null, prefsSnapshot = null
state = const DashboardEditState(isEditing: true);

await ref.read(uspLayoutPreferencesProvider.notifier).initialized; // async gap

// Line 60: post-await bailout
if (!state.isEditing) return;

// Lines 62-70: snapshots captured HERE (after await)
final layoutSnapshot = controller.exportLayout();

If cancelEditMode() fires in the async gap (navigation away before initialized resolves), _exitEditMode(revert: true) runs with state.layoutSnapshot == null — the if (state.layoutSnapshot != null) guard short-circuits, so importLayout + restoreSnapshot are skipped. Any grid changes made between enterEditMode() and initialized resolving will not be reverted.

Trigger: User taps Edit on first launch (prefs cold, initialized is slow) → modifies a card → navigates away before prefs resolve. Grid state is not reverted.

Fix: Capture the layout snapshot synchronously before the await:

final controller = ref.read(uspSliverDashboardControllerProvider);
final earlyLayoutSnapshot = controller.exportLayout();
final earlyPrefsSnapshot = ref.read(uspLayoutPreferencesProvider);
state = DashboardEditState(isEditing: true, layoutSnapshot: earlyLayoutSnapshot, prefsSnapshot: earlyPrefsSnapshot);

await ref.read(uspLayoutPreferencesProvider.notifier).initialized;
if (!state.isEditing) return;

// Refresh prefs snapshot post-initialization
final refreshedPrefs = ref.read(uspLayoutPreferencesProvider);
state = state.copyWith(prefsSnapshot: refreshedPrefs);
controller.setEditMode(true);

W-4 [both reviewers, CARRY-OVER] 🟡Med — No idempotency tests for commitEditMode/cancelEditMode when isEditing == false

test/page/dashboard/providers/dashboard_edit_mode_provider_test.dart

New tests cover re-entrant enter, async-gap cancel, commit-preserves-changes, and multiple enter/commit cycles. None verify that calling commitEditMode() or cancelEditMode() when isEditing == false is a safe no-op (does not corrupt state or throw). The behavioral contract is untested; a future refactor adding a guard would have no regression safety net.

Fix:

test('commitEditMode when not editing is a no-op', () async {
  final container = await createContainer();
  addTearDown(container.dispose);
  await container.read(dashboardEditModeProvider.notifier).commitEditMode();
  final state = container.read(dashboardEditModeProvider);
  expect(state.isEditing, isFalse);
  expect(state.layoutSnapshot, isNull);
});
test('cancelEditMode when not editing is a no-op', () async { ... });

• No security issues — no hardcoded secrets, no injection vectors, no JNAP/USP response handling touched.
• finally block is correct — state always resets, edit mode never gets stranded on I/O failure.
• Re-entrant enterEditMode guard — if (state.isEditing) return + pre-await isEditing: true claim + post-await bail-out is the right pattern for this async lifecycle.
• commitEditMode()/cancelEditMode() naming — explicit names are clearer than exitEditMode({bool save}) inverted-boolean trap.
• Route onExit hardened correctly — try/catch + logger.e + always return true is the correct pattern.
• _openLayoutSettings fix is correct — commitEditMode() for reset/preset_changed preserves the applied change; toggle_off removal confirmed dead path.
• Test coverage for regression — commitEditMode-preserves-changes directly guards against PeterJhong regression.
• layoutSnapshot type tightening — List<Map<String, dynamic>>? correct; consistent with sliver_dashboard 0.9.1.
• linksys_route.dart deletion — LinksysRoute lives on in route_model.dart; file was a standalone re-export that is correctly consolidated.
• PeterJhong regression addressed — _openLayoutSettings now calls commitEditMode() (keep) for reset/preset_changed results.

W-1 · [both reviewers] 🟢High — saveLayout() throw skips restoreSnapshot() on cancel path

lib/page/dashboard/providers/dashboard_edit_mode_provider.dart:92–109

try {
  if (revert) {
    if (state.layoutSnapshot != null) {
      controller.importLayout(state.layoutSnapshot!);  // grid mutated in-memory
      await ref
          .read(uspSliverDashboardControllerProvider.notifier)
          .saveLayout();                                // ← if this throws…
    }
    if (state.prefsSnapshot != null) {
      await ref
          .read(uspLayoutPreferencesProvider.notifier)
          .restoreSnapshot(state.prefsSnapshot!);      // …this is never reached
    }
  }
} finally {
  controller.setEditMode(false);   // ✅ always runs
  state = const DashboardEditState();
}

Trigger: SharedPreferences/localStorage I/O failure (quota exceeded, corrupted storage, write race) during saveLayout() on Android/iOS while cancelling edit mode.

Consequence: importLayout() has mutated the in-memory grid back to snapshot. saveLayout() throws; restoreSnapshot() is skipped. After finally: edit mode exits correctly (✅), but the persisted layout key reflects the last-drag edit value while UspLayoutPreferences retains the edit-mode prefs — the two subsystems are now diverged. prefsSnapshot is wiped in finally, so no retry is possible on next load.

Fix: Separate the two revert operations into independent try/catch blocks:

if (revert) {
  if (state.layoutSnapshot != null) {
    controller.importLayout(state.layoutSnapshot!);
    try {
      await ref.read(uspSliverDashboardControllerProvider.notifier).saveLayout();
    } catch (e, s) {
      logger.e('[Dashboard] saveLayout failed during revert', error: e, stackTrace: s);
    }
  }
  if (state.prefsSnapshot != null) {
    await ref.read(uspLayoutPreferencesProvider.notifier).restoreSnapshot(state.prefsSnapshot!);
  }
}

W-2 · [both reviewers] 🟢High — View button-tap handlers _commitEditMode/_cancelEditMode swallow exceptions silently

lib/page/dashboard/views/usp_sliver_dashboard_view.dart:115–121

void _commitEditMode() async {
  await ref.read(dashboardEditModeProvider.notifier).commitEditMode(); // no try/catch
}

void _cancelEditMode() async {
  await ref.read(dashboardEditModeProvider.notifier).cancelEditMode(); // no try/catch
}

The route guard at route_usp_dashboard.dart:32–44 correctly wraps cancelEditMode() in try/catch(e, s) with logger.e. The two button-tap call sites are inconsistent — any exception escaping _exitEditMode propagates into the void async frame and is silently consumed by Flutter's unhandled-async zone with no log entry.

Note: Reviewer B considers the route-level fix sufficient since navigation-away is the high-stakes path; Reviewer A flags that the button-tap path (explicit user action) should also be logged. Both views have merit — at minimum, adding try/catch + logger.e (without user-facing error) to _cancelEditMode would be consistent with the route guard discipline.

Fix:

void _cancelEditMode() async {
  try {
    await ref.read(dashboardEditModeProvider.notifier).cancelEditMode();
  } catch (e, s) {
    logger.e('[Dashboard] cancelEditMode failed', error: e, stackTrace: s);
  }
}

W-N1 · [both reviewers] 🟡Med — Edit UI visible before controller.setEditMode(true) during async gap

lib/page/dashboard/providers/dashboard_edit_mode_provider.dart:54–72

state = const DashboardEditState(isEditing: true);   // line 54 — UI renders edit mode HERE
                                                      // jiggle, trash zone, edit buttons show

await ref.read(uspLayoutPreferencesProvider.notifier).initialized;  // line 56 — can be slow

if (!state.isEditing) return;                         // line 60

final controller = ref.read(uspSliverDashboardControllerProvider);
// ... snapshot capture ...
controller.setEditMode(true);                         // line 72 — controller enters edit mode ONLY HERE

Trigger: Slow device or first-launch SharedPreferences initialization. uspLayoutPreferencesProvider.initialized awaits SharedPreferences.getInstance() + JSON deserialization — measurably non-zero on first launch.

Consequence: During the async gap (lines 54–72), isEditing == true causes the build to render the full edit-mode UI (jiggle animation, trash zone, action buttons) while DashboardController.setEditMode(true) has not yet been called. Drag and resize gestures are silently rejected by the controller. This is a visible "broken" window on slow devices. Additionally, if cancelEditMode() fires during this gap, _exitEditMode(revert: true) is called with both snapshots null (not yet captured), so the revert body is skipped entirely — and setEditMode(false) is called on a controller that was never put into edit mode.

Fix: Either defer the isEditing = true claim until after initialization completes, or add an isInitializing phase to suppress edit-mode UI until snapshots are captured:

// Option A: delay claim
await ref.read(uspLayoutPreferencesProvider.notifier).initialized;
if (state.isEditing) return;  // double-tap guard still works if re-ordered
state = const DashboardEditState(isEditing: true);
// ... capture snapshots, then controller.setEditMode(true)

W-N2 · [single — security] 🟡Med — Cancel-during-async-gap calls setEditMode(false) on non-edit-mode controller

lib/page/dashboard/providers/dashboard_edit_mode_provider.dart:54–72

Related to W-N1: if cancelEditMode() fires after isEditing=true (line 54) but before controller.setEditMode(true) (line 72), _exitEditMode(revert: true) runs through finally and calls controller.setEditMode(false) on a controller that was never put into edit mode via setEditMode(true). Whether DashboardController.setEditMode(false) handles this gracefully (vs. assert/throw) depends on the package implementation. If the controller maintains internal state, this is an unpaired call.

This is partially mitigated by the if (!state.isEditing) return bail-out at line 60 (enterEditMode resumes and exits early), but _exitEditMode itself has no such guard (see W-3 / Suggestion below). The fix from W-N1 Option A would also resolve this.

S-W5 · [both reviewers, CARRY-OVER] 🟢High — No idempotency tests for commitEditMode/cancelEditMode when isEditing == false

test/page/dashboard/providers/dashboard_edit_mode_provider_test.dart

New tests cover re-entrant enterEditMode and async-gap cancel. No test exercises commitEditMode() or cancelEditMode() called without a preceding enterEditMode. This is needed to validate the idempotency fix proposed in S-W3.

Suggested test:

test('commitEditMode/cancelEditMode are no-ops when not editing', () async {
  final container = await createContainer();
  await container.read(dashboardEditModeProvider.notifier).commitEditMode();
  expect(container.read(dashboardEditModeProvider).isEditing, isFalse);
  await container.read(dashboardEditModeProvider.notifier).cancelEditMode();
  expect(container.read(dashboardEditModeProvider).isEditing, isFalse);
});

S-W3 · [single — architecture] 🟢High — _exitEditMode has no isEditing guard

lib/page/dashboard/providers/dashboard_edit_mode_provider.dart (finally block)

_exitEditMode unconditionally emits state = const DashboardEditState() and calls controller.setEditMode(false) regardless of whether isEditing was true. Calling commitEditMode() or cancelEditMode() when already not editing (double-tap cancel, race with route-exit cancel) causes a spurious Riverpod rebuild on all ref.watch(dashboardEditModeProvider) subscribers.

Fix: Add at top of _exitEditMode:

if (!state.isEditing) return;

S-N3 · [single — architecture] 🟢High — _openLayoutSettings null-result design is implicit

lib/page/dashboard/views/usp_sliver_dashboard_view.dart:499–519

When the settings dialog is dismissed without a 'reset' or 'preset_changed' result (result == null), _openLayoutSettings returns without calling commitEditMode() or cancelEditMode(). The user remains in edit mode. This asymmetry (structural change → auto-exit; no-change dismiss → stay in edit) is intentional but undocumented.

Fix: Add a doc comment explaining the design intent:

/// On 'reset'/'preset_changed': immediately commits to preserve the just-applied
/// structural change. On any other result (null / dialog dismissed): stays in
/// edit mode to allow further fine-tuning before explicit commit or cancel.
Future<void> _openLayoutSettings() async { ... }

S-S1 · [single] 🟡Med — Type erasure cast in settings panel

lib/page/usp_layout_settings_panel.dart:127 (not in this diff, but now inconsistent with it)

final currentIds =
    currentLayout.map((e) => (e as Map)['id'] as String).toSet();

layoutSnapshot in DashboardEditState was tightened from List<dynamic>? to List<Map<String, dynamic>>? by this PR. However, currentLayout here comes from controller.exportLayout() directly (not from the snapshot), which returns List<dynamic>. The as Map + as String double-cast is unguarded — if id is absent or non-String, it throws a TypeError with no fallback.

Fix: Use typed access: (e as Map<String, dynamic>)['id'] as String? with null filtering, or add a null guard.

S-N5 · [single — architecture] ⚪Low — No widget test for _openLayoutSettings commit path

test/…/

The provider-level regression test (commitEditMode preserves layout changes applied during edit) covers the fix at provider level. A widget test for _openLayoutSettings verifying that 'reset'/'preset_changed' results transition isEditing to false while preserving the change would guard against re-introducing the PeterJhong bug at the view layer.

• W-4 (both reviewers) RESOLVED — isEditing = true is claimed synchronously before await initialized, and the post-await bail-out at line 60 (if (!state.isEditing) return) correctly handles the scenario where cancelEditMode() fires during the async gap. Re-entrant enterEditMode double-tap race is fully covered.
• PeterJhong regression FIXED — _openLayoutSettings now calls commitEditMode() (preserve) instead of the old exitEditMode(save: false) (revert) for 'reset' and 'preset_changed' results. The 'toggle_off' case was also removed — confirmed clean: usp_layout_settings_panel.dart no longer pops with 'toggle_off'. Regression test commitEditMode preserves layout changes applied during edit directly guards this path.
• Type-tightening of layoutSnapshot — List<dynamic>? → List<Map<String, dynamic>>? is a correct and conservative tightening consistent with exportLayout()'s contract.
• Route-level error handling — route_usp_dashboard.dart now wraps cancelEditMode() in try/catch(e, s) with logger.e, matching the required discipline for the high-stakes navigation-away path.
• linksys_route.dart deletion — LinksysRoute class is cleanly consolidated into route_model.dart. No dangling imports.
• New tests — Re-entrant guard test, async-gap cancel test, and commitEditMode-preserves-changes regression test all cover the key correctness properties added in this round.